Skip to content

Preserve GVFS locks when commands collide with aliases - #2139

Merged
Tyrie Vella (tyrielv) merged 3 commits into
microsoft:masterfrom
tyrielv:tyrielv/alias-resolution
Nov 5, 2026
Merged

Tyrie Vella (tyrielv) merged 3 commits into
microsoft:masterfrom
tyrielv:tyrielv/alias-resolution

Conversation

@tyrielv

@tyrielv Tyrie Vella (tyrielv) commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem and Context

Git runs the pre-command hook with the name of a built-in command, or with git-<name> when it will run a program or expand an alias.
The hook used a static command inventory and an alias.<name> config query to skip the GVFS lock for aliases.
The inventory omits built-ins such as stash and cannot cover custom programs.
A same-name alias can therefore skip the lock even when Git runs the real command.
Nested Git processes from external programs inherit hook suppression, so they cannot supply the missing lock.
The read-only exemption list also applies to git-<name>, so a program such as git-LOG on PATH runs without a lock.

The config query also misses the [alias "<name>"] command form.
Git 2.56.0.vfs.0.4 does not run post-command for git-<name> before it expands an alias, although its source calls the hook on that path.
So when the hook keeps an outer lock on a genuine alias, the first locking command in that alias waits for its own parent until the user cancels.
The hook must therefore predict Git's choice between a program and an alias.

Ordinary git stash already takes the lock.
This repair addresses command-resolution collisions, not general stash reliability or which built-ins are read-only.

Changes

  • Decide from the raw hook argument. Apply the read-only exemption list only to built-ins, and to Git's git-submodule and git-web--browse programs.
  • Remove the static command inventory.
  • For git-<name>, look for a program as Git for Windows does in path_lookup and lookup_prog: raw PATH entries, "<entry>\git-<name>.exe" with and without .exe, and GetFileAttributesW, which fails for the same files as _waccess. If Git will run a program, take the lock.
  • Otherwise, read the alias config keys with the parent git.exe (git config --name-only --get-regexp ^alias\.). Suppress recursive command hooks, decode the output as UTF-8, and retry once on a transient config access error. Match the keys as Git does: split each key at its last dot, as parse_config_key does. alias.<name>, an empty subsection (alias..<name>), and other dotted names such as alias..review.foo match for any ASCII case. alias.<name>.command matches only the exact name, also when the name starts with a dot. Skip the outer lock only when a key matches.
  • Stop the command with an error when the hook cannot tell what Git will run: the alias keys or the parent git.exe path cannot be read, or a PATH candidate does not fit Git's MAX_PATH buffer.
  • In post-command for git-<name>, release the lock only when the parent Git process owns it. Do not look up aliases or PATH again. If the mount is not reachable, return without an error.
  • Use availability-only locking only for the built-in status, not for a git-status program. Move this check into GitCommandLock, which the unit tests compile.
  • Tell the user to run gvfs status in the new lock-ownership errors.
  • Document how aliases interact with command locks.
  • Add unit tests for built-in and external names, programs found on PATH with Git's path rules, PATH candidates that do not fit Git's buffer, the case rules of each alias form, dot-prefixed subsections, failed alias reads, the read-only list for git-<name>, the availability-only status check, and lock-ownership responses.
  • Add functional tests that run real Git with a lock holder for built-in and external collisions, both alias forms, both dot-prefixed subsection forms, an alias from a config file, a subsection alias with capital letters, nested aliases, command spellings, directories on PATH, and invalid PATH entries. The tests check that a script runs when it collides with an alias.
  • Add a functional test that calls both hooks with a live Git PID and deletes the external program between the calls. The test asserts release before the Git process exits.
  • Add a functional test that calls pre-command with a live Git PID while the global config is locked. The test asserts that the hook stops the command and that the lock stays free.
  • Add a functional test that runs GVFS.Hooks.exe in a folder that has a .gvfs folder but no mount. The test asserts that post-command for git-<name> exits without output, and that post-command for status reports the missing mount.

Validation

The PR compares the time of whole git commands in a mounted dev enlistment, with the hook from master (0889bc6) and the hook from this PR. Both hooks are Release builds with native AOT, as in a release. A temporary test changed the hook path in .git\hooks\*.hooks in each round, and ran each command 60 times with each hook after one warm-up round. The machine has a Snapdragon X Elite (Windows on ARM64, x64 hooks), Git 2.56.0.vfs.0.4, and 31 PATH entries. The table shows the median time in milliseconds.

Command master this PR
git rev-parse HEAD (built-in, no lock) 326 326
git status --porcelain (built-in, lock) 328 334
git perfalias (alias in the local config) 610 621
git perfext (script on PATH) 382 292
git -c alias.stash=version stash list (alias shadows a built-in that master's list does not contain) 497 330
git -c alias.status=version status --porcelain (alias shadows a built-in in master's list) 335 326

Built-in commands show no measurable change. An alias is about 10 ms slower, because the hook checks PATH before it reads the alias keys. An external program is about 90 ms faster, and a built-in that an alias shadows is about 170 ms faster. In both cases, master started git config for each command that was not in its command list.

Known gaps and deliberate decisions

  • Git does not run post-command before alias fallback. If Git ran that callback, the hook could lock whenever it is unsure and release the lock before the alias runs. Until then, a program that Git finds but cannot start causes the alias deadlock. Examples are a program that disappears after the hook checks it, and a script whose #! interpreter does not exist. Cancel the command to end the wait.
  • The alias rules follow current Git. Git versions without [alias "<name>"] command treat that key as the dotted alias <name>.command. The hook matches the newer rule.
  • Some commands stop with an error. If the hook cannot read the alias config, or a PATH entry is too long for Git's path buffer, it stops the command instead of guessing.
  • Custom git-submodule and git-web--browse keep their exemptions. A custom GIT_EXEC_PATH can replace these Git programs. The hook does not check which program Git selects for them.
  • The hook adds work for each git-<name> command. It checks PATH, and it starts git config when no program exists.
  • Hook diagnostics include the failing operation and native error details. They do not create a separate durable log.

Johannes Schindelin (dscho) added a commit to microsoft/git that referenced this pull request Oct 27, 2026
## Problem and Context

`find_hook()` stores the resolved hook path in a static `strbuf`. If the
repository has no gitdir yet, `find_hook()` calls `hook_path_early()`.
That function appends the hooks directory and the hook name to the
buffer, but it does not reset the buffer first. In `v2.53.0.vfs.0.0`,
`find_hook()` called `strbuf_reset(&path)` before either branch. The
rebase onto v2.54.0, which includes upstream b51e238 (`hook: allow
out-of-repo 'git hook' invocations`), rewrote `find_hook()` and dropped
that reset. The gap is present from `v2.54.0.vfs.0.0` through
`vfs-2.56.0`.

As a result, a process that finds `pre-command` before setup cannot find
`post-command` later in the same process. The second lookup produces
`<hooks>/pre-command[.exe]<hooks>/post-command`, so `access()` fails and
Git skips the hook. Built-ins that set up the repository first are not
affected, because `repo_git_path_replace()` resets the buffer. These
paths are affected:

- dashed external commands (`git foo` runs `git-foo`)
- built-ins that do not set up the repository, for example `git version`
- bare `git` with no command (the usage text)
- a dashed external that is not found, after which Git falls back to an
alias

VFS for Git takes a lock in `pre-command` and releases it in
`post-command`. microsoft/VFSForGit#2139 observed a deadlock with
`2.55.0.vfs.0.10`. In that case, a test deleted an external `git-<name>`
after `pre-command` ran and before Git dispatched it. Git then expanded
the alias and started a child Git process. The parent never ran
`post-command`, so the child waited for the parent's lock while the
parent waited for the child.

`postCommand.strategy=worktree-change` is a separate way to skip
`post-command`. VFS for Git does not set it, and the new tests set
`always` explicitly.

## Changes

- `hook.c`: Reset the static path buffer in `find_hook()` before either
the early or the gitdir branch.
- Add a test to `t0401-post-command-hook.sh` that runs a dashed external
command and asserts that `pre-command` and `post-command` both run for
it.
- Add a test to `t0401-post-command-hook.sh` that runs `git version` and
asserts that `pre-command` and `post-command` both run for it.
- Add a test to `t0401-post-command-hook.sh` that uses a `pre-command`
hook to delete a dashed external before Git dispatches it. The test
asserts that Git falls back to the alias and that the hook order is `pre
git-vanish`, `post git-vanish`, `pre rev-parse`, `post rev-parse`.
- The new tests write their own hooks and remove them afterwards, so
they do not depend on the hooks of earlier tests.

Hook consumers will see `post-command` invocations that these releases
skipped. For a dashed external that is not found, the hook receives
`--exit_code=4294967295`, because `exit_code` is `-1` and Git prints it
with `%u`. That formatting is unchanged by this PR. Bare `git` gets
`--exit_code=1`.

## Known gaps and deliberate decisions

- **Relative `core.hooksPath` from a subdirectory**: `hook_path_early()`
resolves a relative `core.hooksPath` against the current directory, not
the worktree root. Early hook lookups from a subdirectory then find no
hooks. This behavior exists before this change and does not change here.
No follow-up is planned yet.
@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/alias-resolution branch 11 times, most recently from c4ef968 to 5472a02 Compare November 4, 2026 22:37
Git runs the pre-command hook with the name of a built-in command, or
with git-<name> when it will run a program or expand an alias. The hook
used a static command inventory and an alias.<name> config query to
skip the lock for aliases. The inventory omitted built-ins such as
stash and did not know custom programs. So a same-name alias could skip
the lock for a command that Git ran as a built-in or a program. The
read-only exemption list also applied to git-<name>, so a program such
as git-LOG on PATH ran without a lock.

The config query also missed [alias "<name>"] command. Git
2.56.0.vfs.0.4 does not run post-command for git-<name> before it
expands an alias. So an outer lock on a genuine alias blocks the first
locking command of that alias until the user cancels.

Decide from the raw hook argument:

- Apply the read-only exemption list only to built-ins, and to Git's
  git-submodule and git-web--browse programs.
- For git-<name>, look for a program as Git for Windows does in
  path_lookup and lookup_prog. Use each raw PATH entry. Check
  "<entry>\git-<name>.exe" with and without ".exe" through
  GetFileAttributesW, which fails for the same files as _waccess.
- If Git will run a program, take the lock.
- Otherwise, read the alias config keys with the parent git.exe
  (git config --name-only --get-regexp ^alias\.). Suppress recursive
  command hooks, decode the output as UTF-8, and retry once on a
  transient config access error. Match the keys as Git does:
  alias.<name>, alias..<name>, and dotted names for any ASCII case, and
  alias.<name>.command only for the exact name. Skip the outer lock
  only when a key matches.
- Stop the command with an error when GVFS cannot tell: the alias keys
  or the path of the parent git.exe cannot be read, or a PATH candidate
  does not fit Git's MAX_PATH buffer.

In post-command for git-<name>, release the lock only when the parent
Git process owns it. Do not look up aliases or PATH again. If the mount
is not reachable, return without an error, because only the mount holds
locks.

Use availability-only locking only for the built-in status, not for a
git-status program.

Remove the static command inventory. Tell the user to run gvfs status
in the new lock-ownership errors. Document how aliases interact with
command locks.

Add unit tests for built-in and external names, programs found on PATH
with Git's path rules, PATH candidates that do not fit Git's buffer,
the case rules of each alias form, failed alias reads, the read-only
list for git-<name>, and lock-ownership responses.

Add functional tests that run real Git with a lock holder for built-in
and external collisions, both alias forms, an alias from a config file,
a subsection alias with capital letters, nested aliases, command
spellings, directories and invalid entries on PATH. The tests check
that a script runs when it collides with an alias. Two functional tests
call the hooks directly with a live Git PID. One deletes the external
program between pre-command and post-command. The other locks the
global config and checks that pre-command stops the command without a
lock.

Assisted-by: GPT-6.1 Sol
Assisted-by: Claude Opus 5.5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
Tyrie Vella (tyrielv) marked this pull request as ready for review November 5, 2026 18:19
Comment thread GVFS/GVFS.Hooks/GitCommandResolver.cs Resolved
Comment thread GVFS/GVFS.Hooks/Program.cs Resolved
The hook removed a leading dot from every alias key after "alias.".
Git removes the dot only when the subsection is empty. Thus Git runs
"alias..review.foo" as the alias ".review.foo", and it runs
[alias ".review"] command as ".review". The hook did not find these
aliases, so it took an outer lock, and the alias command deadlocked.

Split each key at its last dot, as parse_config_key does. An empty
subsection names a plain alias. A "command" variable names the exact
subsection. Any other key names all of the text after "alias.".

Add unit cases for both forms, and add functional tests that run both
forms with real Git while another process holds the lock.

Assisted-by: Claude Opus 5.5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The unit tests do not compile Program.cs, so two decisions there had
no direct tests: which status commands only check the lock, and what
post-command does when the mount is not available.

Move the availability-only check into GitCommandLock, which the unit
tests compile. Add unit tests that check status with --no-lock-index,
status with GIT_OPTIONAL_LOCKS, git-status, STATUS, and other commands.

Add a functional test that runs GVFS.Hooks.exe in a folder that has a
.gvfs folder but no mount. The test checks that post-command for a
git-<name> program exits without output, and that post-command for
status reports that the repo is not mounted.

Assisted-by: Claude Opus 5.5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@KeithIsSleeping

Copy link
Copy Markdown

[KeithBot]: ✅ No issues found · iteration 13 (d1744cb)

Validation

  • ⚠️ Real-git hook e2e (fake mount pipe) — partial
  • ✅ GVFS.UnitTests full suite, head + base
  • ✅ CI run 41931505098: 34/34 jobs pass
  • ✅ Functional build; CLI tests 94/94
  • ⏭️ Mounted GVFS.FunctionalTests locally — not run

Approving.

Also confirmed (raised by others, reproduced by us): T4484948852 (ShiningMassXAcc)

🤖 KeithBot · claude-opus-5.5, gpt-6.1-sol, gemini-4-argon · reply on a thread to dispute

@KeithIsSleeping

Copy link
Copy Markdown

[KeithBot]: ✅ Resolved · iteration 14 (2240a8c)

  • New changes since iteration 13: no new issues

Validation

  • ✅ Real-Git alias oracle: head vs i13
  • ✅ Git 2.56 alias.c config_alias_cb read
  • ✅ CI run 42096498509 at 2240a8c
  • ✅ r1: full GVFS.UnitTests, head + base
  • ✅ New unit tests vs i13 resolver (r1-r3)
  • ✅ r1: e2e real Git, hook, fake mount
  • ✅ r2: hook CLI + availability probes
  • ⏭️ Mounted GVFS.FunctionalTests locally — not run

🤖 KeithBot · claude-opus-5.5, gpt-6.1-sol, gemini-4-argon · reply on a thread to dispute

@tyrielv
Tyrie Vella (tyrielv) merged commit eeb5381 into microsoft:master Nov 5, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants